Add ability to run tests with jQuery Migrate #1774
Conversation
| if ( version === "git" ) { | ||
| url = "http://code.jquery.com/jquery-migrate-" + version; | ||
| } else { | ||
| url = "../../../external/jquery-migrate-" + ( version || "1.12.4" ) + "/jquery-migrate"; |
There was a problem hiding this comment.
There is no jquery-migrate-1.12.4. But we also don't need a default version here since the migrate plugin isn't loaded by default and you have to explicitly specify the version to include.
This also needs to be registered with QUnit so that a UI element is generated. See tests/lib/qunit.js and search for QUnit.config.urlConfig. Since there's a 1:1 mapping between the jQuery version and the jQuery Migrate version, I'd prefer if the migrate config was a boolean. This function should look at the jQuery version to determine the jQuery Migrate version to load.
jQuery 1.x and 2.x -> jQuery Migrate 1.4.1
jQuery 3.x -> jQuery Migrate 3.0.0
jQuery Git -> jQuery Migrate Git
There was a problem hiding this comment.
Updated with your suggestion to resolve jquery-migrate version depending on jquery version. Added QUnit config for it. ?migrate is now a boolean.
| "jquery-3.1.1/LICENSE.txt": "jquery-3.1.1/LICENSE.txt" | ||
| "jquery-3.1.1/LICENSE.txt": "jquery-3.1.1/LICENSE.txt", | ||
|
|
||
| "jquery-migrate-3.0.0/jquery-migrate.js": "jquery-migrate-3.0.0/jquery-migrate.js" |
There was a problem hiding this comment.
We'll want 1.4.1 as well.
| "jquery-3.1.1/LICENSE.txt": "jquery-3.1.1/LICENSE.txt" | ||
| "jquery-3.1.1/LICENSE.txt": "jquery-3.1.1/LICENSE.txt", | ||
|
|
||
| "jquery-migrate-3.0.0/jquery-migrate.js": "jquery-migrate-3.0.0/jquery-migrate.js" |
There was a problem hiding this comment.
Missing the license file.
|
I updated the PR now. Update: the code is ready for a new code review now. |
Run tests with ?jquery=3.1.1&migrate=true to get JQMIGRATE warnings in the console.
| url = "http://code.jquery.com/jquery-migrate-git"; | ||
| } else if ( jqueryVersion[ 0 ] === "3" ) { | ||
| url = "../../../external/jquery-migrate-3.0.0/jquery-migrate"; | ||
| } else { |
There was a problem hiding this comment.
Let's add a condition here for jqueryVersion[ 0 ] === "1" || jqueryVersion[ 0 ] === "2" and then make the else block throw an error so that when jQuery 4 is released, we'll know that we need to check the appropriate jQuery Migrate version.
| @@ -68,6 +69,10 @@ function requireTests( dependencies, noBackCompat ) { | |||
| dependencies.push( "testswarm" ); | |||
| } | |||
|
|
|||
| if ( parseUrl().migrate ) { | |||
There was a problem hiding this comment.
This isn't actually working because parseUrl() doesn't support booleans right now.
This is inspired by #1773 which fixes some JQMIGRATE warnings; but it only fixed a couple ones I stumbled over. In this PR, I added the ability to run all unit tests with jquery-migrate loaded, so that all warnings show up in the console. This will make it easier to systematically fix all migrate warnings, and possibly catch any new ones that are introduced.
Example: Open the browser and run tests: http://localhost:7777/tests/unit/all.html?jquery=3.1.0&migrate=3.0.0
You can then see warnings that appear in the console.